-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Improve diagnostics: update note and add help message #147395
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, some nits as was discussed before
compiler/rustc_expand/messages.ftl
Outdated
.note = if there is a `mod {$name}` elsewhere in the crate already, import it with `use crate::...` instead | ||
expand_module_in_block = | ||
cannot declare a non-inline module inside a block unless it has a path attribute |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As was little discussed here #147314 (comment), in my opinion it worth to use other than "non-inline" word here, this one is not obvious for non native English speakers, so we could use other one that will be more simple
cc @hkBst
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for the review!
I’ve replaced “non-inline module” with “external module” and updated the existing test file name and comments accordingly. I also noticed that “non-inline” is used here
Would it be a good idea to change this message too?
(I’ll squash the commits after the review is finished)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"non-inline modules" is used for a lot of places, I'm not sure if we want to replace them too for now, but at least this one message is need beacuse I think a lot of newcomers can do this mistake and get this error, I believe that start with this place will be good, and then in future we will see if this is necessary to change in other places too
compiler/rustc_expand/messages.ftl
Outdated
cannot declare a non-inline module inside a block unless it has a path attribute | ||
.note = maybe `use` the module `{$name}` instead of redeclaring it | ||
.help = maybe `use` the module `{$name}` instead of redeclaring it | ||
.note = modules are usually placed outside of blocks, at the top level of the file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this note as far as I can say shouldn't say just "modules" but "'verb that describe that this is not an inline module' modules"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reasons github removed part before second modules because I put it in angle brackets, I updated comment
This actually looks good to me, but unfortunately can't rollup since have no rights to do this right now, have to wait when someone else approve it |
Thanks for reviewing! I’ll wait for approval then :) |
027395d
to
002348e
Compare
@bors r=Kivooeo,jackh726 rollup |
I created a topic (#general > Rename "non-inline modules" to "external modules") and we decided to stick with "file modules" rather than "external modules" because of confusion with
And yes, you can open follow up PR or add the changes to this one, both are fine |
…nd update note/help messages
002348e
to
02126ad
Compare
thanks I'll open follow up PR @rustbot ready |
I moved the content from the note to a help message, as it seemed more appropriate there, and then added new information to the note(
Modules are usually placed outside of blocks, at the top level of the file
)!resolve: #147314